Use socket.sendall to send packets #984
Conversation
|
Can one of the admins verify this patch? |
| @@ -238,7 +238,13 @@ def _send_packet(self, packet: dict): | |||
| raw = zlib.compress(utf8bytes(json.dumps(packet))) | |||
| with self._lock: | |||
| try: | |||
| self.output.writelines((PacketManager.SIZE_PACK.pack(len(raw)), raw)) | |||
There was a problem hiding this comment.
Python is supposed to send everything with this call. What exactly is the problem you are seeing?
There was a problem hiding this comment.
We host a very large number of problems, so the handshake packet is quite large (over 100KBs). Everything is fine if both the judge and bridged are run on the same server, but when the judge is moved to a separate server, it fails to handshake.
From what I understand after reading CPython source code, writelines just calls write for every line, which in turn calls socket.send. The document for socket.send clearly notes that "Applications are responsible for checking that all data has been sent; if only some of the data was transmitted, the application needs to attempt delivery of the remaining data." (ref). In our case, raw is so large that socket.send(raw) could not send everything in one pass.
P/s: Before coming up with this fix, we have to monkey patch like this:
self.output.writelines((PacketManager.SIZE_PACK.pack(len(raw)), raw[:100000], raw[100000:]))
which handshakes successfully, so the problem seems to be indeed due to socket.send unable to send everything at once.
There was a problem hiding this comment.
This sounds like a Python bug with the makefile interface to sockets because it's supposed to make sockets look like normal streams and handle incomplete transmission. Out of curiosity, what version are you using and how many problems do you have?
There was a problem hiding this comment.
The judge uses Python 3.9.7. Currently, we have around 9000 problems. You can also reproduce this by adding an extremely long string to the packet:
self._send_packet({'name': 'handshake', 'problems': problems, 'executors': runtimes, 'id': id, 'key': key, 'dummy': 'a' * 1000000000})Btw, where do you get the info that "it's supposed to make sockets look like normal streams and handle incomplete transmission"? I can't seem to find it.
There was a problem hiding this comment.
Because makefile is supposed to return a file object and file objects do not have this weird behaviour in which a write could not complete fully. In this case what I would actually recommend is just getting rid of the file objects and use socket.sendall.
Codecov Report
@@ Coverage Diff @@
## master #984 +/- ##
==========================================
+ Coverage 80.84% 83.44% +2.60%
==========================================
Files 139 139
Lines 4714 4742 +28
==========================================
+ Hits 3811 3957 +146
+ Misses 903 785 -118
Continue to review full report at Codecov.
|
If only some of the data was transmitted, writelines does not attempt to send the remaining data, while sendall continues to send data until either all data has been sent or an error occurs.
hieplpvip
changed the title
|
Could DMOJ please merge this PR, I'm having the same issue :'( |
|
ok to test |
|
Thanks for contributing to DMOJ! |
Currently,
_send_packetdoesn't check if all data has been sent. This is problematic when the number of problems is large, so the packet can't be sent in one pass. This PR fixes that.